Description

POC to assess scope of change/challenges to implement solrmonitor/solrman to make use of the recursive persistent watch.

Since Apache zk v3.6. A new class of watch is introduced which can be persistent and/or recursive.

Before such class of watch, if we want to keep watching a particular data node (as implemented in zkwatcherman for example), we would:

1. call getW() on such node path, which installs a one-time watch on such path
2. If the node has any changes (data change, deletion, addition), we would be notified by zk.Event, then we call getW() again to fetch the actual update (since zk.Event does NOT contain the data) as well as re-installing a new one-time watch

For watching children of a particular node (for example state.json for PRS), it's very similar:

1. call childrenW() on the path (which only returns the child paths, not the data of them), which installs a one-time children watch on it
2. if there are any child changes (deletion, addition), we would be notified by a EventNodeChildrenChanged event, then we call childrenW() again to get the list of children paths and re-install the watch

With permanent watch, we will still NEED to fetch the data again, just that we can call get() instead of getW()

Now recursive watch HAS to be permanent as well. It can become a replacement of data + children watch, more importantly, it could reduce the number of fetch and amount of data tranferred. For example, if we want to watch the child change of state.json. And assume there's a new child entry added, and then a old child entry removed:

• For old watch, we will need to add a children watch on state.json, on first child entry addition, we will need to call childrenW() to get the full list of child paths, and then again on child deletion.
• For persistent and recursive watch, on first child entry addition, we no longer need to fetch the full child path list and the path version is a part of the path, and it's similar for deletion. Take note that we might need to keep the state of the child paths.

Solution

For the zk goclient, there's an open PR (lacking reviews unfortunately) that implements it. Therefore we have cherry picked those changes into https://github.com/patsonluk/zk/commits/cherry-pick-persistent-watch

Now there are 2 major consumers of zk goclient:

1. solrmonitor via zkwatcherman
2. solrman. Which uses zkcli directly for monitoring live nodes, qa nodes and overseer leader. It also uses solrmonitor library to watch collection states. Additionally, it also uses the solrmonitor client to get cluster state from the running solrmonitor cog

This POC in particular focuses on replacing PRS monitoring in solrmonitor/zkwatcherman with permanent recursive watch, which has the best potential for performance improvement. (reduce fetches)

A new branch of gosolr is created https://github.com/fullstorydev/gosolr/tree/patsonluk/zk-persistent-watch that modifies the collections state/PRS monitoring. For each collection, instead of adding a data watch + children watch on state.json. it will only install a single permanent recursive watch. And instead of fetching full child path and updating the collections state with all the entries, it will use the path provided in the zkEvent (so no extra fetching) and update the collection state with it.

solrmonitor should no longer rely on ChildrenChanged (triggered by zk.EventNodeChildrenChanged) for PRS updates, instead it will rely on DataChanged (triggered by zk.EventNodeCreated and zk.EventNodeDeleted) to be notified of PRS entry changes. There's also a new ShouldFetchData function, which would return false for PRS entry, this signals zkwatcherman to NOT fetch the data on zk.EventNodeCreated and zk.EventNodeDeleted as we only care about the path for PRS.

Also zkwatcherman needs special handling for recursive monitoring. For non-recursive monitoring, zkwatcherman always fetches the path (whether get and children) and knows the exact path to fetch. However, for recursive monitoring, it will need to recursively fetch all the children. This however, can be costly (n extra children fetch, which n is # of PRS entries), in the new function MonitorDataRecursive, a parameter initFetchDepth is added to indicate how deep should the children be fetched for init.

To avoid changes to the higher level SolrEventListener, we will still maintain the existing SolrCollectionReplicaStatesChanged(name string, replicaStates map[string]*PerReplicaState) function even for single PRS entry changes (not any worse than before, as n child changes should result as n childrenWatch events before)

Tests

Added extra test into the existing TestPRSProtocol:

1. Added fetch count verification on state.json and PRS entries. This is to verify persistent watch does reduce # of fetch for PRS operations
2. Added a new case to ensure fetched collection/PRS state is correct on existing collection (before solrmonitor starts monitoring it). This is critical as the initial fetch logic on persistent watch is tricky (need to recursively fetch path) unnecessary, found that another test TestCollectionChanges is already checking that

Tested locally with both solrman and solrmonitor referencing the new gosolr hash on https://github.com/fullstorydev/gosolr/commits/patsonluk/zk-persistent-watch . Test operations such as split to ensure it works.

Also wanted to test on solrmonitor (the gosolr) one with --flaky flag, however, it turns out that such flag is only used post init:

, and if we make changes in ZK directly to simulate PRS update, there're no longer any data/children fetching for PRS- as it only reads the PRS path update from notification now (zk.Event)

TODO

This POC focuses only on replacing PRS related zk ops. Therefore only zkwatcherman.MonitorData has been modified (to support recursive watching). Other operations such as cluster props or children watch on collections/live nodes remain the same. Take note that zkwatcherman.MonitorChildren is also unchanged - it's still a one-time watch.

This assumes ALL watches on zkwatcherman.MonitorData are persistent, it is tricky to allow a switch as based on whether the watch is persistent, the zkwatcherman.Callbacks provided by the caller (in this case by solrmonitor) might need to be changed (for example ShouldWatchChildren and ShouldWatchData) as well. Keeping it simple for POC. Update see #59 for extra changes required to support such switch